Added Registration #41

vaibhavsingh97 · 2017-06-28T10:48:17Z

fixes #25
Screenshot after fix:

vaibhavsingh97 · 2017-06-28T10:48:46Z

@tapasweni-pathak @nikhita @jarifibrahim Please review

jarifibrahim

@vaibhavsingh97 This is a huge pull request. You should never make these many changes in one PR. Always try to make atomic commits. :)

One more thing you should be careful about is the indentation settings in your IDE. I believe most of the changes in this PR have been made by the auto-format feature of the IDE.

BTW, I pulled your code and ran it locally. The registration and login process doesn't seem to work.
Please fix it. We cannot merge untill it works.

You just need to make some minor changes then we can merge your code. Good job :)

jarifibrahim · 2017-06-28T14:17:39Z

oshc/.idea/inspectionProfiles/profiles_settings.xml

@@ -0,0 +1,7 @@
+<component name="InspectionProjectProfileManager">


This file is generated by Pycharm and you shouldn't add it.

Please remove this file and all the files in the .idea directory.

jarifibrahim · 2017-06-28T14:25:50Z

oshc/main/static/main/css/app.css

-  transition: opacity 0.7s ease-in-out;
-  font-family: "Montserrat", sans-serif
+    padding: 0;
+    margin: 0;


Most of the changes in this file are related to indentation. Please undo these indentation changes. It's preferable to use a tab size of 2 spaces in css files.

jarifibrahim · 2017-06-28T14:30:13Z

oshc/main/templates/base.html

 							<span class="col-sm-6 left-align">
 								<li>
-									<a href="https://www.facebook.com/opensourcehelpcommunity/"><i class="fa fa-facebook" aria-hidden="true"></i></a>
+									<a href="https://www.facebook.com/opensourcehelpcommunity/"><i


Fix indentation.

jarifibrahim · 2017-06-28T14:32:07Z

oshc/main/templates/registration/password_reset_confirm.html

+                </form>
+
+            {% else %}
+


Unnecessary new line.

jarifibrahim · 2017-06-28T14:33:44Z

oshc/oshc/settings.py

@@ -20,11 +22,10 @@
 SECRET_KEY = 'x1-pogt0-b5owbgq0=ui02-4v)bba!bg&1m8_$)8-&13(907qf'

 # SECURITY WARNING: don't run with debug turned on in production!
-DEBUG = os.getenv("DEBUG", False)
+DEBUG = os.getenv("DEBUG", True)


DEBUG should be set to False.

jarifibrahim · 2017-06-28T14:37:25Z

oshc/oshc/settings.py

@@ -105,3 +120,12 @@
 STATICFILES_DIRS = (
    os.path.join(BASE_DIR, 'main/'),
 )
+
+EMAIL_BACKEND = 'django.core.mail.backends.smtp.EmailBackend'
+EMAIL_USE_TLS = True


Change this and following lines to os.getEnv('EMAIL_USE_TLS). This way it is easier to change the email server and related settings.

jarifibrahim · 2017-06-28T14:38:38Z

oshc/main/templates/base.html

-			</div>
-		</div>
-	</nav>
+<nav class="navbar navbar-inverse navbar-static-top navbar-fixed-top">


Indentation error in the entire file.

jarifibrahim · 2017-06-28T14:42:09Z

oshc/main/templates/base.html

-				<div class="col-xs-12">
-					<div class="social">
-						<ul>
+<div class="container-fluid">


Why change content to container-fluid?

jarifibrahim · 2017-06-28T14:47:49Z

oshc/oshc/urls.py

@@ -4,4 +4,5 @@
 urlpatterns = [
    url(r'^admin/', admin.site.urls),
    url(r'', include('main.urls')),
+    url(r'^accounts/', include('registration.backends.simple.urls')),


You're using the simple backend and you've created the files (email related) for the default backend. You should include registration.backends.default.urls.

vaibhavsingh97 · 2017-06-28T15:29:13Z

@jarifibrahim Thanks for reviewing. Can you please tell me the error?
Can we move to Slack for some discussion?

vaibhavsingh97 · 2017-06-28T16:58:13Z

@jarifibrahim Updated the suggested changes. Please review again

jarifibrahim

The login/registration isn't working. Does it work on your machine?
Also, you might want to squash your commits into a single commit. Ping me on slack if you don't know how to do it.

jarifibrahim · 2017-06-28T17:29:41Z

oshc/main/static/main/css/app.css

 body {
  overflow-x: hidden;
-  padding-top: 50px;
+  margin-bottom: 120px;


This change breaks the layout. You might want to remove it.

jarifibrahim · 2017-06-28T17:31:31Z

oshc/main/templates/base.html

@@ -52,25 +66,25 @@
 									<a href="https://www.youtube.com/channel/UC1_IAby-9me3iICtxuiDL5Q"><i class="fa fa-youtube-play" aria-hidden="true"></i></a>
 								</li>
 							</span>
-							<span class="col-sm-6 backToTop right-align">
+              <span class="col-sm-6 back-to-top right-align">


Indentation error.

jarifibrahim · 2017-06-28T17:49:26Z

oshc/main/static/main/css/app.css

@@ -44,17 +49,31 @@ a {
  border-color: #24292e;
 }

+.container-fluid {


Generally, it is a bad idea to apply styles to bootstrap classes. What if container-fluid is used somewhere else on the same page? It will automatically get padding-left: 0px which is not the intended behavior. You should add your own class/id if you wish to tweak a bootstrap element.

jarifibrahim · 2017-06-28T17:51:57Z

oshc/main/templates/registration/registration_form.html

+                                <button type="submit"
+                                        class="btn btn-lg btn-primary btn-block">{% trans 'Submit' %}</button>
+                            </div>
+


Unnecessary new line.

jarifibrahim · 2017-06-28T17:56:19Z

oshc/main/templates/base.html

-	<script src="{% static "main/js/script.js"%}"></script>
+  <script src="https://ajax.googleapis.com/ajax/libs/jquery/3.2.1/jquery.min.js"></script>
+  <script src="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.7/js/bootstrap.min.js" integrity="sha384-Tc5IQib027qvyjSMfHjOMaLkfuWVxZxUPnCJA7l2mCWNIpG9mGCD8wGNIcPD7Txa" crossorigin="anonymous"></script>
+  <script src="{% static " main/js/script.js " %}"></script>


Change static " main/js/script.js " to static "main/js/script.js"

@vaibhavsingh97 Ping

vaibhavsingh97 · 2017-06-29T05:57:58Z

@jarifibrahim

vaibhavsingh97 · 2017-06-30T14:06:18Z

@tapasweni-pathak Please review and merge.
Steps to run this PR:

python manage.py migrate
python manage.py collectstatic
python manage.py runserver

nikhita · 2017-07-01T15:06:32Z

Thanks for the instructions, @vaibhavsingh97! I checked the changes locally and it works perfectly. 🙌

@jarifibrahim @tapasweni-pathak Since you reviewed the PR, I'll leave it to you to merge it.

jarifibrahim · 2017-07-03T18:20:33Z

Merging this since we have 2 approvals. Well done @vaibhavsingh97

jarifibrahim suggested changes Jun 28, 2017

View reviewed changes

jarifibrahim approved these changes Jul 3, 2017

View reviewed changes

Added registeration

191a12a

jarifibrahim merged commit 6ca6380 into OpenSourceHelpCommunity:develop Jul 3, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added Registration #41

Added Registration #41

vaibhavsingh97 commented Jun 28, 2017

vaibhavsingh97 commented Jun 28, 2017

jarifibrahim left a comment

jarifibrahim Jun 28, 2017

jarifibrahim Jun 28, 2017

jarifibrahim Jun 28, 2017 •

edited

Loading

jarifibrahim Jun 28, 2017

jarifibrahim Jun 28, 2017

jarifibrahim Jun 28, 2017

jarifibrahim Jun 28, 2017

jarifibrahim Jun 28, 2017

jarifibrahim Jun 28, 2017

vaibhavsingh97 commented Jun 28, 2017

vaibhavsingh97 commented Jun 28, 2017

jarifibrahim left a comment

jarifibrahim Jun 28, 2017

jarifibrahim Jun 28, 2017

jarifibrahim Jun 28, 2017

jarifibrahim Jun 28, 2017

jarifibrahim Jun 28, 2017

jarifibrahim Jul 3, 2017

vaibhavsingh97 commented Jun 29, 2017 •

edited

Loading

vaibhavsingh97 commented Jun 30, 2017 •

edited

Loading

nikhita commented Jul 1, 2017

jarifibrahim commented Jul 3, 2017

		@@ -0,0 +1,7 @@
		<component name="InspectionProjectProfileManager">

Added Registration #41

Added Registration #41

Conversation

vaibhavsingh97 commented Jun 28, 2017

vaibhavsingh97 commented Jun 28, 2017

jarifibrahim left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jarifibrahim Jun 28, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vaibhavsingh97 commented Jun 28, 2017

vaibhavsingh97 commented Jun 28, 2017

jarifibrahim left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vaibhavsingh97 commented Jun 29, 2017 • edited Loading

vaibhavsingh97 commented Jun 30, 2017 • edited Loading

nikhita commented Jul 1, 2017

jarifibrahim commented Jul 3, 2017

jarifibrahim Jun 28, 2017 •

edited

Loading

vaibhavsingh97 commented Jun 29, 2017 •

edited

Loading

vaibhavsingh97 commented Jun 30, 2017 •

edited

Loading